-
Notifications
You must be signed in to change notification settings - Fork 58
Update room version to 2.8.4 #1394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the Android Room library from version 2.4.3 to 2.8.4 to fix compatibility issues with modern Android applications. The main change addresses Room's requirement that database queries must not run on the main thread, particularly in constructors.
Key Changes:
- Refactored
OfflineRoomconstructor to remove PRAGMA queries and implement lazy initialization of page size - Updated build infrastructure: Gradle 7.5 → 8.5, AGP 7.3.1 → 8.2.2, minimum SDK 19 → 23
- Simplified native library dependencies by linking maesdk directly via CMake subdirectory
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/OfflineRoom.java | Core fix: moved database queries from constructor to lazy-loaded initPageSize() method; improved field encapsulation |
| lib/android_build/maesdk/build.gradle | Updated Room dependency to 2.8.4 and test dependencies; added Kotlin generation flag |
| lib/android_build/build.gradle | Bumped Android Gradle Plugin from 7.3.1 to 8.2.2 |
| lib/android_build/tools.gradle | Updated compile/target SDK from 32/31 to 34/34, min SDK from 19 to 23 |
| lib/android_build/gradle/wrapper/gradle-wrapper.properties | Updated Gradle wrapper from 7.5 to 8.5 |
| lib/android_build/gradlew | Updated Gradle wrapper scripts with newer template |
| lib/android_build/gradlew.bat | Updated Windows Gradle wrapper script |
| lib/android_build/gradle/wrapper/gradle-wrapper.jar | Binary update for Gradle 8.5 wrapper |
| tools/build-android-aar.sh | New script for building Android AAR files locally with NDK support |
| tests/unittests/PalTests.cpp | Added Android-specific path handling for filesystem tests |
| lib/android_build/app/src/main/cpp/CMakeLists.txt | Changed from importing prebuilt library to building maesdk via subdirectory |
| lib/android_build/app/src/main/java/com/microsoft/applications/events/maesdktest/MainActivity.java | Removed explicit maesdk library loading (handled by CMake now) |
| lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/SDKUnitNativeTest.java | Removed explicit maesdk library loading |
| lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/OfflineRoomUnitTest.java | Added test for lazy-loaded page size; updated deprecated assertion syntax |
| lib/android_build/app/build.gradle | Updated test dependencies to match maesdk module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/OfflineRoom.java
Outdated
Show resolved
Hide resolved
lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/OfflineRoom.java
Outdated
Show resolved
Hide resolved
lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/OfflineRoom.java
Show resolved
Hide resolved
...p/src/androidTest/java/com/microsoft/applications/events/maesdktest/OfflineRoomUnitTest.java
Outdated
Show resolved
Hide resolved
In addition bump gradle and AGP version
…ons/events/OfflineRoom.java Co-authored-by: Copilot <[email protected]>
* Initial plan * Enhance loadPageSize test to verify valid page size Co-authored-by: anod <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: anod <[email protected]>
… static lock (#1396) * Initial plan * Add thread-safety to initPageSize() using double-checked locking Co-authored-by: anod <[email protected]> * Use static lock object for thread-safety in initPageSize() Co-authored-by: anod <[email protected]> * Add concurrent test for thread-safe page size initialization Co-authored-by: anod <[email protected]> * Use ExecutorService instead of raw threads in concurrent test Co-authored-by: anod <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: anod <[email protected]>
f416fd5 to
6093f00
Compare
| defaultConfig { | ||
| minSdkVersion 19 | ||
| targetSdkVersion 31 | ||
| minSdk 23 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this affect devices with older and still supported android version ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't, I have updated it to the minimum required by Google
Modern Google libraries require API level 23 or higher, so any real-world app should target at least that level.
Android 4.4 (API level 19) was released in 2013.
In practice, minSdk 28 (Android 9.0) is the realistic minimum today:
- Outlook and Intune support Android 10.0 (API level 29) and later.
- Teams and Edge support Android 11.0 (API level 30) and later.
| IMPORTED_LOCATION "${SDK_ROOT}/lib/android_build/maesdk/build/intermediates/cmake/debug/obj/${CMAKE_ANDROID_ARCH_ABI}/libmaesdk.so" | ||
| IMPORTED_LOCATION_Release "${SDK_ROOT}/lib/android_build/maesdk/build/intermediates/cmake/debug/obj/${CMAKE_ANDROID_ARCH_ABI}/libmaesdk.so") | ||
| #Add maesdk as a dependency | ||
| add_subdirectory(../../../../maesdk/src/main/cpp maesdk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will create the maesdk library, is this intended ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will build maesdk as part of test app
Currently running test app fails because maesdk library wasn't built when evaluating CmakeLists.
This avoids need to build it separately
Fix #1372 Not Compatible with latest android room 2.7.2
Unblock android apps for upgrading Room library